Skip to content

fix: code quality and safety improvements - #749

Open
saurabhhhcodes wants to merge 1 commit into
GitMetricsLab:mainfrom
saurabhhhcodes:fix/github_tracker-82853
Open

fix: code quality and safety improvements#749
saurabhhhcodes wants to merge 1 commit into
GitMetricsLab:mainfrom
saurabhhhcodes:fix/github_tracker-82853

Conversation

@saurabhhhcodes

@saurabhhhcodes saurabhhhcodes commented Aug 2, 2026

Copy link
Copy Markdown

Summary by CodeRabbit

  • Bug Fixes
    • Improved activity feed error handling by detecting unsuccessful responses before processing data.
    • Preserved existing loading and error-state behavior when activity requests fail.
    • Maintained equivalent validation behavior for permitted cross-origin requests.

@netlify

netlify Bot commented Aug 2, 2026

Copy link
Copy Markdown

Deploy Preview for github-spy ready!

Name Link
🔨 Latest commit 80c8326
🔍 Latest deploy log https://app.netlify.com/projects/github-spy/deploys/6a6f30d01cd3de0008f08579
😎 Deploy Preview https://deploy-preview-749--github-spy.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The backend changes the CORS allowlist check to use includes. The activity feed now rejects unsuccessful HTTP responses before parsing JSON.

Changes

CORS validation

Layer / File(s) Summary
CORS allowlist membership
backend/server.js
The CORS origin check uses allowedOrigins.includes(origin) with unchanged behavior.

Activity fetch error handling

Layer / File(s) Summary
Activity response validation
src/components/ActivityFeed.tsx
fetchEvents rejects non-OK responses before JSON parsing and uses the existing error handler.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: mehul-m-prajapati

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request has no description and omits all required template sections, including testing details and change type. Add the required sections, link the related issue, describe the changes, document testing, add screenshots when applicable, and select the change type.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately identifies the changes as code quality and safety improvements.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

backend/server.js

typescript-eslint does not support TS 7.0.
Please see https://devblogs.microsoft.com/typescript/announcing-typescript-7-0/#running-side-by-side-with-typescript-6.0 to run typescript-eslint using the TS 6 API.
See also typescript-eslint/typescript-eslint#10940 for tracking typescript-eslint's support for TS >=7.1

Oops! Something went wrong! :(

ESLint: 9.39.5

Error: typescript-eslint does not support TS 7.0.
at Object. (/node_modules/.pnpm/typescript-eslint@8.65.0_eslint@9.39.5_supports-color@7.2.0__supports-color@7.2.0_typescript@7.0.2/node_modules/typescript-eslint/dist/index.js:52:11)
at Module._compile (node:internal/modules/cjs/loader:1830:14)
at Object..js (node:internal/modules/cjs/loader:1961:10)
at Module.load (node:internal/modules/cjs/loader:1553:32)
at Module._load (node:internal/modules/cjs/loader:1355:12)
at wrapModuleLoad (node:internal/modules/cjs/loader:255:19)
at loadCJSModuleWithModuleLoad (node:internal/modules/esm/translators:326:3)
at ModuleWrap. (node:internal/modules/esm/translators:231:7)
at ModuleJob.run (node:internal/modules/esm/module_job:437:25)
at async node:internal/modules/esm/loader:639:26

src/components/ActivityFeed.tsx

typescript-eslint does not support TS 7.0.
Please see https://devblogs.microsoft.com/typescript/announcing-typescript-7-0/#running-side-by-side-with-typescript-6.0 to run typescript-eslint using the TS 6 API.
See also typescript-eslint/typescript-eslint#10940 for tracking typescript-eslint's support for TS >=7.1

Oops! Something went wrong! :(

ESLint: 9.39.5

Error: typescript-eslint does not support TS 7.0.
at Object. (/node_modules/.pnpm/typescript-eslint@8.65.0_eslint@9.39.5_supports-color@7.2.0__supports-color@7.2.0_typescript@7.0.2/node_modules/typescript-eslint/dist/index.js:52:11)
at Module._compile (node:internal/modules/cjs/loader:1830:14)
at Object..js (node:internal/modules/cjs/loader:1961:10)
at Module.load (node:internal/modules/cjs/loader:1553:32)
at Module._load (node:internal/modules/cjs/loader:1355:12)
at wrapModuleLoad (node:internal/modules/cjs/loader:255:19)
at loadCJSModuleWithModuleLoad (node:internal/modules/esm/translators:326:3)
at ModuleWrap. (node:internal/modules/esm/translators:231:7)
at ModuleJob.run (node:internal/modules/esm/module_job:437:25)
at async node:internal/modules/esm/loader:639:26


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/components/ActivityFeed.tsx`:
- Around line 36-37: Update ActivityFeed’s fetch flow to track request errors
separately from the events list: when res.ok is false, set the error state and
render a failure message instead of “No activity found,” while preserving
loading cleanup. Add a regression case in ActivityFeed.test.tsx covering a
non-OK response and asserting the failure message is shown.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 382e2aed-dd23-4ece-865d-3cd96c20dbb3

📥 Commits

Reviewing files that changed from the base of the PR and between 53f820b and 80c8326.

📒 Files selected for processing (2)
  • backend/server.js
  • src/components/ActivityFeed.tsx

Comment on lines +36 to +37
if (!res.ok) throw new Error("Request failed");
const data = await res.json();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Do not report HTTP failures as an empty feed.

When res.ok is false, the error reaches the catch block, which only clears loading. If events is empty, the component renders No activity found for a failed request. Add an error state and render a failure message instead. Add a regression test in src/components/ActivityFeed.test.tsx for a non-OK response.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/ActivityFeed.tsx` around lines 36 - 37, Update ActivityFeed’s
fetch flow to track request errors separately from the events list: when res.ok
is false, set the error state and render a failure message instead of “No
activity found,” while preserving loading cleanup. Add a regression case in
ActivityFeed.test.tsx covering a non-OK response and asserting the failure
message is shown.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant